Provide context directory for extensions #1276

modulo11 · 2024-01-24T07:19:55Z

Summary

This PR implements the extension layer RFC by providing one of the following context directory to the kaniko build during the extension phase:

$CNB_OUTPUT_DIR/context
$CNB_OUTPUT_DIR/context.build and/or $CNB_OUTPUT_DIR/context.run

More details and examples can be found in the RFC.

Release notes

Provide and allow extensions to write into a shared or specific context directory.

Context

loewenstein · 2024-01-25T17:15:06Z

@natalieparellano once the spec changes are in, this would be worth it a look. Or already now, if there is review feedback that requires changes.

natalieparellano · 2024-01-29T23:03:41Z

@modulo11 @loewenstein thank you for this - apologies for being slow to review the changes. It looks like there might be a couple of tests to fix, but I'll take a deeper look tomorrow to see if I could offer more useful feedback.

buildpack/buildcontext.go

phase/extender.go

phase/generator.go

buildpack/generate.go

phase/extender.go

loewenstein · 2024-02-06T20:21:05Z

@modulo11 @loewenstein thank you for this - apologies for being slow to review the changes. It looks like there might be a couple of tests to fix, but I'll take a deeper look tomorrow to see if I could offer more useful feedback.

@modulo11 did we have a look for what causes the test failures?

codecov · 2024-02-09T08:11:25Z

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (be5e24b) 64.47% compared to head (ca4f6db) 64.65%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1276      +/-   ##
==========================================
+ Coverage   64.47%   64.65%   +0.18%     
==========================================
  Files         100      101       +1     
  Lines        6873     6949      +76     
==========================================
+ Hits         4431     4492      +61     
- Misses       2041     2051      +10     
- Partials      401      406       +5

Flag	Coverage Δ
os_linux	`64.12% <79.35%> (+0.18%)`	⬆️
os_windows	`56.91% <28.27%> (-0.26%)`	⬇️
unit	`64.12% <79.35%> (+0.18%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

phase/generator.go

buildpack/buildcontext.go

natalieparellano

@modulo11 thank you for this. In general this looks good. I still need to go through the unit tests a bit more carefully to understand what is tested.

It would be awesome if we could add an acceptance test (by adding a new fixture or symlinking to the child fixtures here). I actually think (without updating the fixtures) the current acceptance tests are going to start failing when we add 0.13 to the list of supported Platform APIs here. You could go ahead and do that as part of this PR if you want. When we add an (unreleased) API version we typically add a row to the table in the README, as shown in this PR.

LMK if I can be of any assistance here, especially with the acceptance test. Running our suite can be a bit tricky.

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com> Signed-off-by: Pavel Busko <pavel.busko@sap.com>

Co-authored-by: Pavel Busko <pavel.busko@sap.com> Signed-off-by: Johannes Dillmann <j.dillmann@sap.com>

Co-authored-by: Ralf Pannemans <ralf.pannemans@sap.com> Signed-off-by: Pavel Busko <pavel.busko@sap.com>

pbusko · 2024-02-13T08:46:23Z

@natalieparellano we've addressed all the comments and introduced additional acceptance test with the extensions contexts

buildpack/generate.go

natalieparellano · 2024-02-14T15:08:04Z

phase/generator_test.go

-						t.Fatalf("Incorrect error: %s\n", err)
-					}
-				})
+		when("using the platform API 0.13", func() {


I'm not sure it's worth having a whole separate block for the 0.13 tests - it looks like if we run the 0.12 block with generator.PlatformAPI = api.MustParse("0.13") the only two tests that fail are:

it("copies Dockerfiles and extend-config.toml files to the correct locations", func() {

descCondition: "a run.Dockerfile declares a new base image (only) and no run.Dockerfiles follow", descResult: "returns the referenced base image, and sets extend to false",

It might make our future lives easier to just conditionally set a variable for "dockerfile parent dir" depending on the Platform API and use that in the tests

phase/generator.go

natalieparellano · 2024-02-14T15:34:29Z

phase/extender_test.go

-				desc := func(b bool) string {
-					if b {
-						return "rebasable"
+		when("using platform API 0.13", func() {


Similar comment to #1276 (comment), I wonder if we could just make vars for "A build.Dockerfile path", "A build BuildContext" (etc) and make this more of a table test.

natalieparellano · 2024-02-14T15:38:52Z

@pbusko @modulo11 thank you for this - I left a few comments but they are mostly nits about test organization. LMK what is your bandwidth for addressing these as it would be nice to cut an 0.19.0 RC soon. I'd be okay leaving #1276 (comment) and #1276 (comment) for later refactoring if we want to merge this one soon.

Signed-off-by: Philipp Stehle <philipp.stehle@sap.com>

phil9909 · 2024-02-15T13:36:48Z

@natalieparellano I addressed the two small comments.
On the tests: I have some ideas how to change them, but there is no obviously superior way of doing this. So I would prefer to do this in a follow-up PR as you suggested, so we can start a fresh discussion there. Maybe even have two competing PRs.

loewenstein

LGTM

modulo11 requested a review from a team as a code owner January 24, 2024 07:19

This was referenced Jan 24, 2024

add support for pack to extend base images using daemon buildpacks/pack#1791

Draft

Making builpack image extensions feature non-experimental buildpacks/spec#378

Closed

modulo11 force-pushed the extension-contexts branch from 5428f32 to 650a3bf Compare January 25, 2024 14:17

natalieparellano reviewed Jan 30, 2024

View reviewed changes

buildpack/buildcontext.go Outdated Show resolved Hide resolved

phase/extender.go Outdated Show resolved Hide resolved

phase/generator.go Outdated Show resolved Hide resolved

buildpack/generate.go Outdated Show resolved Hide resolved

phase/extender.go Show resolved Hide resolved

modulo11 requested review from loewenstein and natalieparellano February 2, 2024 13:47

natalieparellano added this to the lifecycle 0.19.0 milestone Feb 7, 2024

natalieparellano assigned modulo11 Feb 8, 2024

modulo11 force-pushed the extension-contexts branch 2 times, most recently from cbc65a2 to 63b1b73 Compare February 9, 2024 07:59

modulo11 force-pushed the extension-contexts branch from 63b1b73 to aef7fd2 Compare February 9, 2024 09:06

natalieparellano reviewed Feb 9, 2024

View reviewed changes

phase/generator.go Outdated Show resolved Hide resolved

natalieparellano reviewed Feb 9, 2024

View reviewed changes

phase/generator.go Outdated Show resolved Hide resolved

natalieparellano reviewed Feb 9, 2024

View reviewed changes

buildpack/buildcontext.go Outdated Show resolved Hide resolved

natalieparellano reviewed Feb 9, 2024

View reviewed changes

c0d1ngm0nk3y force-pushed the extension-contexts branch from aef7fd2 to 61cecce Compare February 12, 2024 13:41

pbusko force-pushed the extension-contexts branch 4 times, most recently from dfbbe62 to c455f40 Compare February 13, 2024 08:41

pbusko and others added 5 commits February 13, 2024 09:44

Provide context folder for extensions

9396684

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com> Signed-off-by: Pavel Busko <pavel.busko@sap.com>

Address PR feedback

1501d18

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com> Signed-off-by: Pavel Busko <pavel.busko@sap.com>

Split tests by platform API

5c6cbd3

Co-authored-by: Pavel Busko <pavel.busko@sap.com> Signed-off-by: Johannes Dillmann <j.dillmann@sap.com>

Add unit test for FindContexts

f098a71

Co-authored-by: Ralf Pannemans <ralf.pannemans@sap.com> Signed-off-by: Pavel Busko <pavel.busko@sap.com>

acceptnance tests

e7adefb

Co-authored-by: Ralf Pannemans <ralf.pannemans@sap.com> Signed-off-by: Pavel Busko <pavel.busko@sap.com>

pbusko force-pushed the extension-contexts branch from c455f40 to e7adefb Compare February 13, 2024 08:45

natalieparellano reviewed Feb 14, 2024

View reviewed changes

Address PR feedback

ca4f6db

Signed-off-by: Philipp Stehle <philipp.stehle@sap.com>

natalieparellano approved these changes Feb 15, 2024

View reviewed changes

natalieparellano merged commit 7bdfb33 into buildpacks:main Feb 15, 2024
10 checks passed

loewenstein reviewed Feb 15, 2024

View reviewed changes

pbusko deleted the extension-contexts branch February 15, 2024 21:32

This was referenced Feb 16, 2024

refactor phase/extender_test.go #1297

Merged

refactor phase/generator_test.go #1301

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide context directory for extensions #1276

Provide context directory for extensions #1276

modulo11 commented Jan 24, 2024

loewenstein commented Jan 25, 2024

natalieparellano commented Jan 29, 2024

loewenstein commented Feb 6, 2024

codecov bot commented Feb 9, 2024 •

edited

Loading

natalieparellano left a comment

pbusko commented Feb 13, 2024

natalieparellano Feb 14, 2024

natalieparellano Feb 14, 2024

natalieparellano commented Feb 14, 2024

phil9909 commented Feb 15, 2024

loewenstein left a comment

Provide context directory for extensions #1276

Provide context directory for extensions #1276

Conversation

modulo11 commented Jan 24, 2024

Summary

Release notes

Related

Context

loewenstein commented Jan 25, 2024

natalieparellano commented Jan 29, 2024

loewenstein commented Feb 6, 2024

codecov bot commented Feb 9, 2024 • edited Loading

Codecov Report

natalieparellano left a comment

Choose a reason for hiding this comment

pbusko commented Feb 13, 2024

natalieparellano Feb 14, 2024

Choose a reason for hiding this comment

natalieparellano Feb 14, 2024

Choose a reason for hiding this comment

natalieparellano commented Feb 14, 2024

phil9909 commented Feb 15, 2024

loewenstein left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 9, 2024 •

edited

Loading